Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Refactor: rename query as queryAsFlow to avoid confusion when using the API. #89

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Refactor: rename query as queryAsFlow to avoid confusion when using the API. #89

wants to merge 1 commit into from

Conversation

driessamyn
Copy link
Owner

No description provided.

@Copilot Copilot bot review requested due to automatic review settings March 7, 2025 08:44
Copilot
Copilot AI reviewed Mar 7, 2025
View reviewed changes

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.
@qodo-merge-pro-for-open-source Qodo Merge Pro for Open Source
Copy link

PR Code Suggestions ✨

Explore these optional code suggestions:

CategorySuggestion                                                                                                                                    Impact
General
Fix misleading documentation comment

The documentation example still uses the term "user list" which implies a List
rather than a Flow. Update the comment to better reflect the Flow nature of the
API.

coroutines/src/main/kotlin/net/samyn/kapper/coroutines/KapperKotlinFlowQueryFun.kt [17-42]

 /**
  * Execute a SQL queryAsFlow and map the results to a Flow of instances of the specified class.
  *
  * This function uses reflection to automatically map the result set columns to the properties of the specified class.
  * For advanced mappings, use the overloaded version with a custom `mapper` function.
  *
  * **Example**:
  * ```kotlin
  * // Assuming we have a data class
  * data class User(val id: Int, val name: String)
  *
  * // Fetch all users where "active" is true
  * val users: Flow<User> = connection.queryAsFlow(
  *     sql = "SELECT id, name FROM users WHERE active = :active",
  *     "active" to true
  * )
  *
- * // Iterate over the user list and print each user
+ * // Process the user flow and print each user
  * users.collect { println(it) }
  * ```
  *
  * @param sql The SQL queryAsFlow to execute.
  * @param args Optional key-value pairs representing named parameters to substitute into the queryAsFlow.
  * @return The queryAsFlow result as a [Flow] of [T] instances.
  * @throws java.sql.SQLException If there's a database error.
  */
  • Apply this suggestion
Suggestion importance[1-10]: 5

__

Why: The suggestion correctly identifies that the documentation comment still refers to "user list" which is inconsistent with the Flow-based API. Changing to "user flow" better reflects the nature of the API and improves documentation clarity.

Low
  • More

@sonarqubecloud SonarQubeCloud
Copy link

sonarqubecloud bot commented Mar 7, 2025

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
100.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 7, 2025

Test Results

 37 files  ±0   37 suites  ±0   27s ⏱️ ±0s
299 tests ±0  299 ✅ ±0  0 💤 ±0  0 ❌ ±0 
357 runs  ±0  357 ✅ ±0  0 💤 ±0  0 ❌ ±0 

Results for commit a1cf25d. ± Comparison against base commit b87bd96.

This pull request removes 38 and adds 37 tests. Note that renamed tests count towards both. 

net.samyn.kapper.internal.QueryParserTest ‑ [5] 
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [10] LONGNVARCHAR, LONGNVARCHAR, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$859/0x00007f295c4ccee0@7f8c1f21
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [11] INSTANT, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$723/0x00007f295c471bb8@6aff86cc
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [11] LONGVARCHAR, LONGVARCHAR, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$860/0x00007f295c4cd100@396d65ba
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [12] DATE, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$724/0x00007f295c472000@4d6db023
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [12] NCHAR, NCHAR, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$861/0x00007f295c4cd320@60f99f2c
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [13] NCLOB, NCLOB, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$862/0x00007f295c4cd540@12dd6574
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [14] NVARCHAR, NVARCHAR, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$863/0x00007f295c4cd760@14321d76
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [15] ROWID, ROWID, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$864/0x00007f295c4cd980@5a4dabd7
…

net.samyn.kapper.internal.QueryParserTest ‑ [5] 

net.samyn.kapper.internal.SQLTypesConverterTest ‑ [10] LONGNVARCHAR, LONGNVARCHAR, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$801/0x00007f67a04b2260@18de1523
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [11] INSTANT, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$609/0x00007f67a03d69b8@463ea06a
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [11] LONGVARCHAR, LONGVARCHAR, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$802/0x00007f67a04b2480@7d8393f
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [12] DATE, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$610/0x00007f67a03d6bd8@3d5a51fd
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [12] NCHAR, NCHAR, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$803/0x00007f67a04b26a0@d8494ed
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [13] NCLOB, NCLOB, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$804/0x00007f67a04b28c0@7a544817
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [14] NVARCHAR, NVARCHAR, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$805/0x00007f67a04b2ae0@673e109b
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [15] ROWID, ROWID, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$806/0x00007f67a04b2d00@9b168f9
net.samyn.kapper.internal.SQLTypesConverterTest ‑ [16] SQLXML, SQLXML, net.samyn.kapper.internal.SQLTypesConverterTest$Companion$$Lambda$807/0x00007f67a04b2f20@84df32e
…

@github-actions GitHub Actions
Copy link

github-actions bot commented Mar 7, 2025

Code Coverage

File Coverage [100.00%]
coroutines/src/main/kotlin/net/samyn/kapper/coroutines/DatasourceWithConnection.kt 100.00%
coroutines/src/main/kotlin/net/samyn/kapper/coroutines/KapperKotlinFlowQueryFun.kt 100.00%
Total Project Coverage 99.32%

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

Copilot code review Copilot Copilot left review comments

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@driessamyn